Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix compile of arbitrary feature. #505

Merged
merged 2 commits into from
Sep 8, 2023
Merged

Fix compile of arbitrary feature. #505

merged 2 commits into from
Sep 8, 2023

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Sep 7, 2023

The arbitrary feature is currently broken, because the Arbitrary derive macro assumes std is present:

$ cargo build --features arbitrary
   Compiling euclid v0.22.9 (/Users/kpreid/Projects/rust/contributing/euclid)
error[E0433]: failed to resolve: use of undeclared crate or module `std`
  --> src/angle.rs:26:42
   |
26 | #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
   |                                          ^^^^^^^^^^^^^^^^^^^^ use of undeclared crate or module `std`
   |
   = note: this error originates in the derive macro `arbitrary::Arbitrary` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0425]: cannot find value `RECURSIVE_COUNT_Angle` in this scope
  --> src/angle.rs:26:42
   |
26 | #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
   |                                          ^^^^^^^^^^^^^^^^^^^^ not found in this scope
   |
   = note: this error originates in the derive macro `arbitrary::Arbitrary` (in Nightly builds, run with -Z macro-backtrace for more info)

This PR replaces the sole use of that macro with a manual implementation, and adds CI testing for the feature so it won't break again.

@kpreid
Fix compile of arbitrary feature.
f7f8884 
`derive(arbitrary::Arbitrary)` uses paths from `std`, which is
incompatible with `#![no_std]`. The simplest path forward is to use a
manual implementation instead of the derive.

An alternative to this strategy would be to change the conditional
`no_std` to be `not(any(test, feature = "arbitrary")`, but this seems
simpler and more robust.
@kpreid
Add arbitrary feature to CI configurations.
edd93d0 
Also move `bytemuck` into the matrix like all the other features are.
@jdm jdm enabled auto-merge September 8, 2023 03:10
@jdm jdm added this pull request to the merge queue Sep 8, 2023
Merged via the queue into servo:master with commit be0a341 Sep 8, 2023
18 checks passed
@kpreid kpreid deleted the arb branch July 15, 2024 05:12
kpreid added a commit to kpreid/euclid that referenced this pull request Jul 15, 2024
@kpreid
Remove derive feature from arbitrary.
c1ff18b 
It has not been used since commit be0a341
aka PR <servo#505>.
kpreid added a commit to kpreid/euclid that referenced this pull request Jul 15, 2024
@kpreid
Remove dependency on derive feature of arbitrary.
f344007 
It has not been used since commit be0a341
aka PR <servo#505>.
@kpreid kpreid mentioned this pull request Jul 15, 2024
Merged
kpreid added a commit to kpreid/euclid that referenced this pull request Jul 15, 2024
@kpreid
Remove dependency on derive feature of arbitrary.
eeaede0 
It has not been used since commit be0a341
aka PR <servo#505>.
github-merge-queue bot pushed a commit that referenced this pull request Jul 16, 2024
@kpreid
Remove dependency on derive feature of arbitrary. (#530)
57e1332 
It has not been used since commit be0a341
aka PR <#505>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm jdm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kpreid @jdm